build: run basic feature matrix in CI#313
Conversation
8ad29d4 to
b056a64
Compare
|
Oh hm, looks like all features flags is broken right now. I'll update the patch to drop that till we can fix it. |
241e5ec to
6243472
Compare
|
Was able to fix up the tor feature deps just enough to get Ideally the MSRV build check was a little more feature thorough, but I think it is good enough to prevent any big regressions. |
|
At first glance I think a couple things are going on here. Could you potentially make a new PR for the I would imagine you can just cherry-pick them onto a new branch pretty easily. |
Hm yea, I forget if the bumps are necessary to get CI to work, but if that is the case I'll rebase them down. |
f131669 to
eade7c8
Compare
|
Ok, was able to move out the dependency bumps except for the tor one which is essential for the second patch eade7c8 |
Extending the "build" step to run all feature flags, as well as the existing default. Could possible add more flag combos in the future if things get complicated, but this should cover most cases for now. This helps ensure that the different flag combos exposed by the library actually build for consumers. If the feature flag matrix begins to get more complex, we could look into a more specific tool like cargo-hack, but for now simplicity is fine. Switched to the optional dependency syntax which ensures that the (unused in our case) implicit features of the optional dependencies are not created. This cleans up the library interface for consumers, only our explicit flags are exposed. Dropping the "check" CI step since build covers all of it. If build gets really slow in the future, we can add check back as a form of "fast failure", but as it is today this is just duplicating work and slowing CI down.
eade7c8 to
5fed91d
Compare
|
Hate to hold this up forever, but since it touches the "database" feature, it seems like an opportunity to get rid of that horrible feature name. In the case we implement a new storage backend, like |
I think we should follow convention and name the feature after the dependency, so in this case |
|
Works for me |
|
Going to just fold this into #322 |
First patch focuses on running some feature flag combinations, second patch fixes the tor dep so we can also run all feature flags.
Extending the "build" step to run all feature flags, as well as the existing default. Could possible add more flag combos in the future if things get complicated, but this should cover most cases for now. This helps ensure that the different flag combos exposed by the library actually build for consumers. If the feature flag matrix begins to get more complex, we could look into a more specific tool like cargo-hack, but for now simplicity is fine.
Switched to the optional dependency syntax which ensures that the (unused in our case) implicit features of the optional dependencies are not created. This cleans up the library interface for consumers, only our explicit flags are exposed.
Dropping the "check" CI step since build covers all of it. If build gets really slow in the future, we can add check back as a form of "fast failure", but as it is today this is just duplicating work and slowing CI down.
Requires bumping the tor dependencies due to a break in the pulled in time dependency. This is a conservative upgrade so we don't run into issues with the sqlite dependency, however the MSRV with the tor feature is still above the projects, 1.70.0.